-
Notifications
You must be signed in to change notification settings - Fork 767
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tabbed content widget #2059
tabbed content widget #2059
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple or so small documentation things, but also a couple of errors that seem like they're easily fixed.
I feel this also deserves some dedicated unit tests too given that this is a fairly involved container, testing the messages and the like, and also testing things like TabbedContent
inside TabbedContent
, etc (I did try that "by hand" and seems fine once the issue around pulling back the ContentSwitcher
was fixed).
Overall though this is really nice! Looks and feels nice and smooth.
Co-authored-by: Dave Pearson <davep@davep.org>
Co-authored-by: Dave Pearson <davep@davep.org>
Random thought while playing with this some more: I feel like it might be a good idea to have |
@davep Added some tests. The new pilot.click is awesome. |
|
||
| Name | Type | Default | Description | | ||
| -------- | ----- | ------- | -------------------------------------------------------------- | | ||
| `active` | `str` | `""` | The `id` attribute of the active tab. Set this to switch tabs. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last nit... as of 2ede38f this needs to show that it can be None
.
@willmcgugan Looks good! I suspect I'll be giving pilot.click a workout with the menu stuff too; looking forward to it. For the sake of completeness I think I'd be temped to add a tabs-within-tabs test (given that caused an error initially and could possibly be a source of regression at some point), but that's your call. Looking good! Looks very mergeable to me now! |
TabbedContent
widgetTabPane
widgetget_child_by_type
methodScreen.Recording.2023-03-17.at.09.18.49.mov